-
Notifications
You must be signed in to change notification settings - Fork 154
chore(layers): bundle assets from source when testing #1710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Kudos, SonarCloud Quality Gate passed!
|
Ah, I have discovered exactly the same issue when working on #1703 because we can't test layer without having a release. Now we can combine both solutions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outstanding refactoring! I was working on improving the shell script, but you took it a step further, well done!
I was tempted to add the packages and adjust the function code here, but I will address it in a separate PR.
Thanks, glad you agree! I thought about that too but the issue was assigned to you and saw you were already pretty far ahead with the tests so you should take credit for that work. |
Description of your changes
This PR introduces some changes to the way the Lambda Layers are built when running integration tests. The changes should be backward compatible with the current release process, which should work as before for the most part.
Below a matrix of the changes and the areas they are supposed to impact:
Bundling Script
Before this PR we had a bash script present in the
.github/scripts/
folder that had to be run before deploying the layers or running tests.This was fine in the CI environment since it was only an additional step. When working locally however we didn't have any automated way of running it and contributors/maintainers had to remember to run it before being able to run the integration tests for the
layers
workspace.This PR removes this script in favor of using CDK local bundling. The
Code.fromAsset()
method that we use to reference the layer content accepts two arguments: the location of the asset and a bundling configuration object.Even though this is not really well documented, it's possible to force the bundling to happen locally and without Docker. This is exactly what the
NodejsFunction
L3 construct does under the hood and also what described here in passing.The command looks something like this, but with more
execSync
commands:With this method we don't have to maintain two scripts and the build process is always run automatically as part of the synth process 🚀 .
Lambda Function
Before this release we were performing the following checks:
package.json
file of one utility (Logger) matches the expected versionColdStart
)This PR changes the first and last actions to:
package.json
files of each utility present in the layer matches the expected oneThe first one is mainly for an excess of caution, however the second was needed because most of the methods in the Tracer are treated as non-critical and as such they are not meant to throw if they don't succeed. By logging the content of a segment we can test that the actions we perform actually happened.
Build Steps
As mentioned in the linked issue, before this PR we didn't have any means to test the Powertools utilities in a Layer without making a release. This PR introduces a new CDK context variable called
BuildFromSource
that allows to tell the CDK construct to build the packages from sources before deploying the Lambda Layer (defaults tofalse
).This behavior is set to
true
in the integration tests but defaults tofalse
in the release pipeline. WhenBuildFromSource
istrue
, the build process runsnpm run build -w packages/<utility>
andnpm pack -w packages/<utility>
, then uses the resulting tarballs to install the utility in the layer folder.This not only allows us to test the code we are working on in the integration tests, but also serves as an early canary for the build/pack process itself.
Related issues, RFCs
Issue number: closes #1709
Checklist
Breaking change checklist
Is it a breaking change?: NO
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.